Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(arcgis): support arcgis content #335

Merged
merged 11 commits into from
Jan 16, 2024
Merged

feat(arcgis): support arcgis content #335

merged 11 commits into from
Jan 16, 2024

Conversation

mspivak-actionengine
Copy link
Collaborator

No description provided.

@aws-amplify-us-east-2 AWS Amplify (us-east-2)
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-335.d3nof3l3x2sso4.amplifyapp.com

@dsavinov-actionengine
Copy link

Why not make this "ready for review"?
I don't see a reason why not.

belom88
belom88 requested changes Dec 21, 2023
View reviewed changes
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
src/utils/arcgis.ts Outdated Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/components/modal-dialog/modal-dialog.tsx Outdated Show resolved Hide resolved
src/pages/debug-app/debug-app.tsx Outdated Show resolved Hide resolved
src/utils/arcgis.ts Outdated Show resolved Hide resolved
src/components/common.tsx Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
@mspivak-actionengine mspivak-actionengine marked this pull request as ready for review December 25, 2023 10:02
belom88
belom88 requested changes Dec 25, 2023
View reviewed changes
src/components/common.tsx Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/redux/slices/arcgis-content-slice.ts Outdated Show resolved Hide resolved
src/utils/arcgis.spec.ts Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
belom88
belom88 requested changes Jan 8, 2024
View reviewed changes
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Show resolved Hide resolved
src/components/layers-panel/layers-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated Show resolved Hide resolved
src/components/layers-panel/arcgis-control-panel.spec.tsx Outdated Show resolved Hide resolved
belom88
belom88 approved these changes Jan 16, 2024
View reviewed changes
Copy link
Collaborator

@belom88 belom88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left minor comments. Can be quickly fixed and merged.

src/utils/arcgis.ts Outdated
@@ -97,3 +101,49 @@ export const arcGisRequestLogout = async () => {
}
return await updateSessionInfo();
};

class ArcGisContentClass implements ArcGisContent {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class ArcGisContentClass implements ArcGisContent {
class ArcGisContent implements IArcGisContent {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated
overflow: hidden;
`;

const TableHeaderRow = styled.tr``;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary component. Use tr in place

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated
year: "numeric",
});
return formatter.format(date);
type Column = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move outside of the component

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated
columnName?: string;
};

const columns: Column[] = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move outside to component

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated
return (
<TableHeaderCell
width={column.width}
key={sortDataColumnName ? sortDataColumnName : ""}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add an id field to the columns config to use it in JSX?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/components/layers-panel/arcgis-import-panel/arcgis-import-panel.tsx Outdated
<TableRowCell
width={column.width}
fontWeight={column.fontWeight}
key={`${dataColumnName ? dataColumnName : ""}${contentItem.id}`}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mspivak-actionengine mspivak-actionengine merged commit e531f73 into ms/arcgis-connect Jan 16, 2024
3 checks passed
@mspivak-actionengine mspivak-actionengine deleted the ms/arcgis-content-ui branch January 16, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsavinov-actionengine dsavinov-actionengine dsavinov-actionengine left review comments

@belom88 belom88 belom88 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mspivak-actionengine @dsavinov-actionengine @belom88